Skip to content

chore(compliance): drop agent_test_history + last_test_* — final cleanup#4274

Open
EmmaLouise2018 wants to merge 1 commit intoEmmaLouise2018/unification-pr4-collapse-last-test-columnsfrom
EmmaLouise2018/unification-pr5-final-cleanup-drop-test-history
Open

chore(compliance): drop agent_test_history + last_test_* — final cleanup#4274
EmmaLouise2018 wants to merge 1 commit intoEmmaLouise2018/unification-pr4-collapse-last-test-columnsfrom
EmmaLouise2018/unification-pr5-final-cleanup-drop-test-history

Conversation

@EmmaLouise2018
Copy link
Copy Markdown
Contributor

PR 5 (final cleanup) of the #4247 unification stack. Stacked on #4268#4264#4263#4250.

Summary

Drops the dual-write infrastructure now that PR #4250 writes canonical, PR #4264 backfilled history, and PR #4268 derives `last_test_*` via the `agent_context_with_latest_test` view. Closes #4247.

Pre-merge gate (load-bearing — destructive migration)

DO NOT MERGE until each is satisfied:

The migration is destructive and irreversible. The S3 export from gate (4) is the recovery path.

What changes

  • Migration 474:
    • Redefines `agent_context_summary` view without references to the dropped table/columns (history counts now derive from `agent_compliance_runs`)
    • Drops `agent_contexts.last_test_*` columns
    • Drops `agent_test_history` table
    • Refreshes `agent_context_with_latest_test` so the `ac.*` projection no longer carries the removed columns
  • `agent-context-db.ts`:
    • Removes `recordTest`, `getTestHistory`, `getLatestTestForUser`
    • Removes `AgentTestHistory` and `RecordTestInput` interfaces
    • `update()` no longer SETs `last_test_*`; refetches via `getById()` after the UPDATE so derived view fields stay populated for callers
  • `evaluate_agent_quality`: drops the third-party `recordTest()` call. Non-owner runs are session-scoped — they return results in the response and do not persist
  • `run_storyboard`: drops its `recordTest()` call. Single-storyboard runs are session-scoped (canonical writes for single storyboards would over-state coverage)

Behavior change (named per Brian's bar)

  • Third-party `evaluate_agent_quality` runs against someone else's agent no longer leave any persistent state. Matches the "owner-only canonical writes" policy from Unify compliance state: every storyboard run writes to one canonical path (heartbeat + Addie + dashboard tests) #4247
  • `run_storyboard` runs (any caller) no longer leave persistent state. The dashboard's "tested at" timestamps for an org reflect only `evaluate_agent_quality` runs (full comply suite). Single-storyboard runs are exploratory tooling
  • The `AgentContext` shape on read remains unchanged — `last_test_*` fields still appear in the response, sourced from the view

S3 export script (operator runs before migration)

fly ssh console -a adcp-docs
cd /app && cat > /app/export-third-party-history.mjs <<'EOF'
import pg from 'pg';
import { writeFile } from 'node:fs/promises';
const c = new pg.Client({ connectionString: process.env.DATABASE_URL });
await c.connect();
const r = await c.query(\`
  SELECT id, agent_context_id, scenario, overall_passed,
         steps_passed, steps_failed, total_duration_ms,
         summary, dry_run, brief, triggered_by, user_id,
         steps_json, agent_profile_json, started_at, completed_at
    FROM agent_test_history
   WHERE user_id IS NULL
\`);
const jsonl = r.rows.map(row => JSON.stringify(row)).join('\\n');
await writeFile('/app/agent_test_history_third_party.jsonl', jsonl);
console.log(\`Exported \${r.rowCount} third-party rows\`);
await c.end();
EOF
node /app/export-third-party-history.mjs
# upload /app/agent_test_history_third_party.jsonl to S3 cold storage
# commit upload evidence (SHA256 + S3 path) to ops runbook

Stacked on

Merge order: #4250#4263#4264#4268 → this PR.

Closes #4247.

Test plan

  • `tsc --noEmit -p server/tsconfig.json` clean
  • Migration 474 applies cleanly on staging (after migration 473 from PR feat(compliance): derive agent_context.last_test_* from canonical runs #4268)
  • Post-migration: `SELECT * FROM information_schema.tables WHERE table_name = 'agent_test_history'` returns 0 rows
  • Post-migration: `agent_context_summary` and `agent_context_with_latest_test` views still serve queries cleanly
  • Post-migration: `SELECT last_test_scenario, last_test_passed, last_test_summary, last_tested_at, total_tests_run FROM agent_context_with_latest_test LIMIT 5` returns view-derived values for owner-triggered rows
  • Smoke after deploy: third-party `evaluate_agent_quality` returns results but does NOT create rows in any table
  • Smoke after deploy: owner `evaluate_agent_quality` creates a row in `agent_compliance_runs` with `triggered_by='owner_test'` AND `triggered_org_id` set

PR 5 of the #4247 unification stack. Removes the dual-write
infrastructure now that PR #4250 writes canonically, PR #4264
backfilled history, and PR #4268 derives last_test_* via the
agent_context_with_latest_test view.

Migration 474 (gate-protected — destructive):
- Redefines agent_context_summary without agent_test_history refs
- Drops agent_contexts.last_test_* columns
- Drops agent_test_history table
- Refreshes agent_context_with_latest_test

Code:
- agent-context-db.ts drops recordTest, getTestHistory,
  getLatestTestForUser, AgentTestHistory, RecordTestInput
- update() refetches via getById() so derived view fields stay populated
- evaluate_agent_quality drops the third-party recordTest call
- run_storyboard drops its recordTest call

Behavior change: third-party evaluate_agent_quality and any
run_storyboard call no longer persist registry state. Matches the
owner-only canonical writes policy from #4247.

Stacked on #4268#4264#4263#4250.
@bokelley
Copy link
Copy Markdown
Contributor

bokelley commented May 9, 2026

Code review (expert pass): block — final cleanup needs three fixes.

1. Verify server/src/db/org-merge-db.ts is clean.
org-merge-db.ts:637 has a comment referencing agent_test_history. The merge flow may rely on ON DELETE CASCADE from agent_contexts. With the table dropped, runtime impact needs explicit confirmation — read the surrounding block before merge and either remove the dead comment or adjust the merge logic.

2. S3 export script is in the PR body, not in server/src/scripts/.
Per repo convention (see feedback_prod_runnable_scripts_path.md), prod-runnable scripts live under server/src/scripts/ and must call initializeDatabase() before getPool(). The body's heredoc bypasses both. Move it to a tracked file before merge.

3. Wrap migration 474 in an explicit transaction.
The destructive ordering (drop view → drop columns → drop table → recreate view) inside one migration file means a partial failure leaves the DB un-bootable. Wrap in BEGIN; … COMMIT;, or split into two files (drop, then recreate-view). Postgres DDL is mostly transactional but mixing column drops with view recreates needs the explicit boundary for safe rollback.

Nit (non-blocking):
update() does UPDATE-then-getById (two round-trips). UPDATE … RETURNING with an explicit column list (excluding the dropped ones) would be one query. Premature simplification — fine to defer.

@bokelley
Copy link
Copy Markdown
Contributor

bokelley commented May 9, 2026

Blocker fixes prepared — needs author to apply

All three blockers from @bokelley's review have been worked out. I can't push directly to EmmaLouise2018/* branches, so the diffs are below for @EmmaLouise2018 to apply. The nit (UPDATE … RETURNING) is deferred per reviewer guidance.


Fix 1 — org-merge-db.ts: remove dead agent_test_history reference

Two spots. The cascade concern is moot (no child tables remain); logic is unchanged.

-    // UNIQUE(organization_id, agent_url): keep primary's row on
-    // conflict so its agent_test_history (ON DELETE CASCADE) survives;
-    // secondary's history is removed when its row is deleted.
+    // UNIQUE(organization_id, agent_url): on conflict keep primary's
+    // row; secondary's conflicting rows are deleted below (no child
+    // tables remain after migration 474 dropped agent_test_history).
-        `${agentContextsDeleted.rows.length} duplicate agent_contexts from secondary org were deleted (primary already had the same agent_url) — their test history was removed`
+        `${agentContextsDeleted.rows.length} duplicate agent_contexts from secondary org were deleted (primary already had the same agent_url)`

Fix 2 — migration 474: explicit transaction

 -- the S3 export from gate (4), not pg_dump.
 
+BEGIN;
+
 -- ── Phase 1: drop the dependent view …
 ) AS run_counts ON TRUE;
+
+COMMIT;

Fix 3 — S3 export script: server/src/scripts/export-third-party-test-history.ts (new file)

import { writeFile } from 'node:fs/promises';
import { initializeDatabase, getPool, closeDatabase } from '../db/client.js';
import { getDatabaseConfig } from '../config.js';

async function main(): Promise<void> {
  const dbConfig = getDatabaseConfig();
  if (!dbConfig) { console.error('DATABASE_URL is required'); process.exit(1); }
  initializeDatabase(dbConfig);
  const pool = getPool();

  const result = await pool.query(`
    SELECT id, agent_context_id, scenario, overall_passed,
           steps_passed, steps_failed, total_duration_ms,
           summary, dry_run, brief, triggered_by, user_id,
           steps_json, agent_profile_json, started_at, completed_at
      FROM agent_test_history
     WHERE user_id IS NULL
  `);

  const jsonl = result.rows.map((row) => JSON.stringify(row)).join('\n');
  const outPath = '/app/agent_test_history_third_party.jsonl';
  await writeFile(outPath, jsonl, 'utf-8');
  console.log(`Exported ${result.rowCount} third-party rows to ${outPath}`);
  console.log('Next: upload to S3 cold storage and commit SHA256 + S3 path to ops runbook.');
  await closeDatabase();
}

main().catch((err) => { console.error(err); process.exit(1); });

The full doc-comment header and usage instructions are in the prepared commit. The initializeDatabase() → getPool() ordering follows repair-dangling-primary-orgs.ts and the other scripts in that directory.


Session: https://claude.ai/code/session_0195XGWfSj96CJCJcxhxUX6m


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants